Skip to content

VIF: Fix out of bounds read caused by broken alignment logic - #14853

Open
chaoticgd wants to merge 1 commit into
PCSX2:masterfrom
chaoticgd:vif_padding
Open

VIF: Fix out of bounds read caused by broken alignment logic#14853
chaoticgd wants to merge 1 commit into
PCSX2:masterfrom
chaoticgd:vif_padding

Conversation

@chaoticgd

@chaoticgd chaoticgd commented Aug 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Description of Changes

Removes some broken alignment logic in nVifUnpack.

Rationale behind Changes

The correct way to align the value up would be (size + 3) & ~0x3, not (size + 4) & ~0x3, but from checking around the code it seems like that logic isn't required anyway since the VU_Thread class makes sure everything is aligned itself.

I was getting the following error while running PCSX2 under ASAN:

    #0 0x55fbe1f2b3fe in __asan_memcpy (/home/thomas/projects/pcsx2/build/bin/pcsx2-qt+0xa393fe) (BuildId: 3a503ce477c319bbb93e9a3d8c857b9def34cba1)
    #1 0x55fbe265b5eb in VU_Thread::Write(void const*, unsigned int) /home/thomas/projects/pcsx2/pcsx2/MTVU.cpp:324:2
    #2 0x55fbe265b5eb in VU_Thread::VifUnpack(vifStruct&, VIFregisters&, unsigned char const*, unsigned int) /home/thomas/projects/pcsx2/pcsx2/MTVU.cpp:476:2
    #3 0x55fbe2809b4a in int nVifUnpack<1>(unsigned char const*) /home/thomas/projects/pcsx2/pcsx2/Vif_Unpack.cpp:364:14
    #4 0x55fbe27b200a in void vifTransferLoop<1>(unsigned int*&) /home/thomas/projects/pcsx2/pcsx2/Vif_Transfer.cpp:46:9
    #5 0x55fbe27b0ae5 in bool vifTransfer<1>(unsigned int*, int, bool) /home/thomas/projects/pcsx2/pcsx2/Vif_Transfer.cpp:65:2
    #6 0x55fbe27b0ae5 in VIF1transfer(unsigned int*, int, bool) /home/thomas/projects/pcsx2/pcsx2/Vif_Transfer.cpp:117:9
    #7 0x55fbe279b35b in vif1SetupTransfer() /home/thomas/projects/pcsx2/pcsx2/Vif1_Dma.cpp
    #8 0x55fbe279b35b in vif1Interrupt() /home/thomas/projects/pcsx2/pcsx2/Vif1_Dma.cpp:422:4
    #9 0x55fbe26dd9f4 in TESTINT(unsigned char, void (*)()) /home/thomas/projects/pcsx2/pcsx2/R5900.cpp:253:3
    #10 0x55fbe26dd9f4 in _cpuTestInterrupts() /home/thomas/projects/pcsx2/pcsx2/R5900.cpp:277:3
    #11 0x55fbe26dd9f4 in _cpuEventTest_Shared() /home/thomas/projects/pcsx2/pcsx2/R5900.cpp:428:4
    #12 0x55fbe2feaba5 in recEventTest() /home/thomas/projects/pcsx2/pcsx2/x86/ix86-32/iR5900.cpp:400:2
    #13 0x7b045ef00008  (<unknown module>)

0x55fbe6b0d610 is located 48 bytes before global variable 'mfifoVIF1transfer()::masked_tag' defined in '/home/thomas/projects/pcsx2/pcsx2/Vif1_MFIFO.cpp:201' (0x55fbe6b0d640) of size 16
0x55fbe6b0d610 is located 0 bytes after global variable 'vif1SetupTransfer()::masked_tag' defined in '/home/thomas/projects/pcsx2/pcsx2/Vif1_Dma.cpp:172' (0x55fbe6b0d600) of size 16
SUMMARY: AddressSanitizer: global-buffer-overflow (/home/thomas/projects/pcsx2/build/bin/pcsx2-qt+0xa393fe) (BuildId: 3a503ce477c319bbb93e9a3d8c857b9def34cba1) in __asan_memcpy
Shadow bytes around the buggy address:
  0x55fbe6b0d380: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 00 00 00 00
  0x55fbe6b0d400: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 00 00 00 00
  0x55fbe6b0d480: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 00 00 00 00
  0x55fbe6b0d500: 01 f9 f9 f9 01 f9 f9 f9 04 f9 f9 f9 00 00 00 00
  0x55fbe6b0d580: 04 f9 f9 f9 00 00 f9 f9 00 00 00 00 04 f9 f9 f9
=>0x55fbe6b0d600: 00 00[f9]f9 00 00 00 00 00 00 f9 f9 00 00 00 00
  0x55fbe6b0d680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9
  0x55fbe6b0d700: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x55fbe6b0d780: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x55fbe6b0d800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x55fbe6b0d880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
Thread T4 (CPU Thread) created by T0 here:
    #0 0x55fbe1f10df5 in pthread_create (/home/thomas/projects/pcsx2/build/bin/pcsx2-qt+0xa1edf5) (BuildId: 3a503ce477c319bbb93e9a3d8c857b9def34cba1)
    #1 0x7f0493b05c3c in QThread::start(QThread::Priority) (/home/thomas/projects/pcsx2/deps/lib/libQt6Core.so.6+0x305c3c) (BuildId: 274b75bf25690bee0ae8628feab6096c0fed5032)

Suggested Testing Steps

Try a bunch of VU intensive games.

Did you use AI to help find, test, or implement this issue or feature?

I asked ChatGPT if it could see anything wrong with it after opening it, apart from that no.

@github-actions github-actions Bot added the VIF label Aug 16, 2026
@Mrlinkwii Mrlinkwii added this to the Release 2.10 milestone Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

VIF

Projects

None yet

Milestone

Release 2.10

Development

Successfully merging this pull request may close these issues.

2 participants

@chaoticgd @Mrlinkwii